Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Less type safety: remove Handler::Msg assoc. type #309

Merged
merged 47 commits into from
Apr 26, 2022
Merged

Less type safety: remove Handler::Msg assoc. type #309

merged 47 commits into from
Apr 26, 2022

Conversation

dhardy
Copy link
Collaborator

@dhardy dhardy commented Apr 25, 2022

When is type safety not a good thing? This PR removes the Handler::Msg type and Response::Msg variant, replacing this with a new variadic message queue.

  • Add EventMgr::push_msg, try_pop_msg (+ boxed versions)
  • Remove Response::Update and Updatable trait
  • Remove Handler::Msg, Response::Msg, VoidMsg, #[derive(VoidMsg)
  • Remove #[widget(update = f)], #[widget(map_msg = f)], etc. (#[widget] attr on fields no longer accepts parameters)
  • Button, CheckBox etc. closures no longer return Option<M> but push directly via mgr.push_msg(..)
  • Remove IndexedList, IndexedColumn, IndexedRow, GenericList
  • Remove Response::Select; add SelectMsg
  • Remove Response::Scrolled, Pan and Focus variants; new Scroll enum
  • Replace dyn WidgetConfig with dyn Widget
  • Remove SendEvent; replace with EventMgr::send method (widgets no longer handle event routing)
  • Warn in log if a message is unhandled, with value of message (debug only)
  • Add Handler::handle_message: called on parent whenever a child emits a message
  • Add List::on_message and Grid::on_message; optionally call closure with child index when child emits a message
  • Remove Driver::get
  • *Data traits: replace update_handles with update_on_handles (pass &WidgetId in and let data register dependencies), update method takes &mut EventMgr and notifies of update instead of returning an UpdateHandle
  • Add *Data::handle_message, called by view widget when a child emits a message (replacement for Response::Update and Updatable)

There are also a few fixes / minor behaviour changes:

  • ComboBox widget: if menu entries have accelerator keys, these are now only usable when the menu is open (same as other menus)
  • ComboBox: fix highlighting of first entry
  • ComboBox: constructor no longer takes active parameter; all entries have an associated message (of the same type)
  • examples/mandlebrot: enable tab focus of main widget (allows full keyboard control)
  • Splitter widget: fix child index to WidgetId assignment
  • Fix widget derive: implement all methods
  • Slider widget: clamp handle to detents
  • Handler::handle renamed to handle_event
  • Prettier WidgetHeirarchy output
  • Fix weird width/height interaction in RowSetter for columns

Effects and motivation

With over 40 commits and approx 2000 line insertions and 3000 deletions, this is not a small PR, and a significant step in trait revision (the rough topic for v0.11). Effects:

  • There is no longer a compile-time error when forgetting to handle a message from a child widget. Instead, there is only a warning in the log. This is a definite regression, albeit not too important I think (some more testing will be required when building UIs).
  • Multiple message types may be emitted by the same widget (or by a parent alongside a child's message), and multiple concurrent messages may be placed on the stack. Minor improvement.
  • One less type parameter. dyn Widget is now possible without parametrizing the message type. Less type parameters to worry about implies simpler code and simpler error messages.
  • Event routing is now done by the library; no more widget-specific SendEvent impls! As a consequence, the Handler trait gained handle_message, handle_unused and scroll methods.
  • Data models/view widgets changed significantly with regards to handling user input, with roughly the same capability and possibly slightly better code.

Effects on user code not using data models is limited, mostly to not needing to specify message types or handler annotations (e.g. #[widget(use_msg = start)]).

Closes #11, #257.

Bugs

Tab order broken in examples filter-list and data-list-view when paging. Warnings are logged, e.g.:

[2022-04-26T08:28:01Z WARN  kas_core::event::manager] Widget ListView#052 cannot find path to #0529310
[2022-04-26T08:28:02Z WARN  kas_core::event::manager] Widget ListView#052 cannot find path to #0529220

I will leave this to a future PR since this one is otherwise ready.

MSRV

Bump min Rust version to 1.58, only for the new format syntax (embed identifiers).

This is impossible with the new message model. No big loss.
This is not emitted by their children
This breaks ComboBox and automatic updating of data
through view widgets (to be fixed later).
This is a big revision to event handling, allowing multiple
simultaneous messages from children and removing the need to
parametrize many widgets over the message type.

Incomplete: new Handler::on_message method not called yet.

Remove Response::Msg, VoidMsg, #[derive(VoidMsg)]
Replace event::ChildMsg with IndexMsg, SelectMsg
Remove Updatable trait; add SingleData::on_message, etc.
Remove #[widget(flatmap_msg=.., map_msg=.., use_msg=.., discard_msg)]
make_widget!: remove -> message-type bound
AdaptWidget: remove map_void_msg, map_msg_discard; adjust map_msg
Button, etc...: event closure no longer returns Option<M>
Grid, List: now push IndexMsg given a child message
Rename widgets::view::SelectMsg → SelectionMsg
New Scroll enum, EventMgr::set_scroll and Handler::scroll methods
This gets lots of things working again, but many bugs remain.
This prevents keys activating by combobox menu items from
doing anything unless the menu is already open, which is
the same behaviour as used by menus.
This enables useful output in the case of unhandled messages.
Slider now holds handle to its detents.
Slider now takes nav-focus on click.
Same for SingleData, ListData, MatrixData
@dhardy dhardy merged commit e32b1c1 into master Apr 26, 2022
@dhardy dhardy mentioned this pull request Sep 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bad error msg when widget response payload not convertible to parent's type
1 participant